-
Notifications
You must be signed in to change notification settings - Fork 387
Use Navigation 3 #504
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use Navigation 3 #504
Conversation
15a34ae to
d9221a3
Compare
| ) | ||
| } | ||
|
|
||
| AnimatedContent( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interested to understand why you didn't use NavDisplay here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, it's what I originally did, you can see the move away from it in this commit: ddeeb64
It's just slightly simpler to do "in-screen" switching between the four main screens here than to run a nested NavDisplay for the main screen within the global one. It took a couple lines of extra code to make sure saved state is handled correctly with this setup, but it was much much easier than I expected.
Another option would have been to move those 4 screens into the global navigation, and either
- Have them each contain their own copy of the bottom nav bar - this just feels... weird, and it would have taken extra work to make the bottom bar transition nicely between screens as far as I understand.
- Move the bottom bar outside the global
NavDisplayand hide/show it based on the current screen - but this global bottom bar feels overkill when only 4 out of the dozens of screens will use it.
Also, there's special navigation behaviour between the 4 main screens (from any of them, back goes back to the first tab, then quits the app). Bolting that logic onto the global backstack would also seem like an unnecessary complication.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, you're using a NavDisplay for all other screens except MainScreen which uses an AnimatedContent to switch between the "main" screens all of which share a bottom nav bar.
Although it feels weird, I think option 1 is actually the option you want to aim for here (although not necessarily in this PR). Wrapping the bottom nav bar in a shared element will lock it in place for the main screens and animate it out for all others. To avoid the duplication of the bottom nav bar inside each entry, you could create a custom entry decorator (e.g. NavBarDecorator) that is only applied to MainRoutes. This would wrap the content of those main entries with the nav bar.
Note that this approach will currently only work if you don't use any scenes that render multiple entries (e.g. list-detail) because the nav bar will be rendered inside the entry, not at the scene level. Scene decorators (coming soon to Nav3) will cover this use case.
For the special navigation behavior you mention, this is the same as the behavior in our multiple back stacks recipe here: https://github.com/android/nav3-recipes/blob/main/app/src/main/java/com/example/nav3recipes/multiplestacks/NavigationState.kt#L74.
The real advantage of moving everything inside a single NavDisplay is you can consolidate your navigation logic. At the moment you have 2 separate navigation systems: one for the main NavDisplay and one inside MainScreen. Another advantage is that you could show the bottom nav bar on non-top-level screens - at the moment you always have to go back from a sub-screen to show the nav bar.
To be clear, I think your current implementation is fine, and there's definitely something nice about having a single back stack where it's always [MainScreen, other keys], but for the reasons above, plus if you ever wanted to support adaptive layouts (desktop maybe?), moving to multiple back stacks and a single NavDisplay is the way to go.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the detailed answer, that's a great overview! I think the way to go then is to keep this setup for this PR, and experiment with a decorator and merging the main screens into the "global" NavDisplay setup on another one to see how it works and feels.
The real advantage of moving everything inside a single NavDisplay is you can consolidate your navigation logic. At the moment you have 2 separate navigation systems: one for the main NavDisplay and one inside MainScreen.
It's true that we have two systems right now, but since the one in MainScreen is completely contained (in a single, fairly short file), it actually seems perfectly fine to me. I quite like the idea that the "global" setup using Nav3 doesn't have to know about any of the special things that MainScreen includes (except we're messing with a back handler a little, but that also doesn't require the code outside the screen to know about it).
Another advantage is that you could show the bottom nav bar on non-top-level screens - at the moment you always have to go back from a sub-screen to show the nav bar.
True, although we don't require this anywhere for now (or foreseeable future).
if you ever wanted to support adaptive layouts (desktop maybe?), moving to multiple back stacks and a single NavDisplay is the way to go
We are definitely thinking of going adaptive for large screens at some point, it's currently in the works. So if nothing else, that'll make us revamp the navigation story a bit, I'm sure 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dturner Nice topic!
Could you share your opinion, what is the best approach if I want the same but with a small addition: I want to switch home tabs as like viewpager's pages (by left/right gestures)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For me, it looks like a self-contained widget with its internal behavior. I wouldn't want to mix it with an application navigation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not? It's just adding a key to a list (the application back stack). Unless the UI component has its own back handler, in which case that's a strong signal that it is navigable content, and the back handler should be linked to the application back, rather than handling its own state using navigation events.
Is the issue more to do with encapsulation? As in, you don't want to define a NavEntry at the application level when it feels like it should be scoped to an individual screen or substack? If so, that's a legitimate concern but can be solved with modularization and/or substack-specific entry providers.
That said, if you consider the SearchBar to be a toolbar, and the back arrow doesn't trigger a "back" navigation event but instead just collapses the toolbar then I agree that this isn't navigable content. You're just changing the visibility of a UI component.
Thanks for the discussion btw - it's interesting to define the line of what is/isn't navigable content.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is a point of my confusion:
the back arrow doesn't trigger a "back" navigation event but instead just collapses the toolbar
The same way, I can describe a behavior of the bottom bar: this is just a switching between main screen UI parts. There is no even back navigation between them.
Or even dialogs: this is an additional part of some screen, and it is possible to hide by a back click.
So, I would summarize it as every case is unique, and every app has the own approaches to a navigation design.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same way, I can describe a behavior of the bottom bar: this is just a switching between main screen UI parts. There is no even back navigation between them.
That sounds like a strange UX if you tap on one of the bottom bar items, then tap back and it exits the app rather than going back to the previously selected bottom bar item. That's not what the KotlinConf app does.
Or even dialogs: this is an additional part of some screen, and it is possible to hide by a back click.
If you hide it by tapping back, that's navigable content. Another way of thinking about this is that the system back event is a global application event, why would you handle it at the sub-screen level? Also, to be clear a NavEntry can be displayed on top of another NavEntry. This is done at the scene level using SceneOverlay. This is how bottom sheets and dialogs are implemented in Nav3.
So, I would summarize it as every case is unique, and every app has the own approaches to a navigation design.
Yeah, that's why navigation is a mess in so many apps ;-) If you abide by the rule of "make everything a NavEntry if you can navigate back from it" then navigation becomes a lot easier to manage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That sounds like a strange UX if you tap on one of the bottom bar items, then tap back and it exits the app rather than going back to the previously selected bottom bar item
Usually an app doesn't go back to the previous screen but goes to the "main" tab and then exits. So, this is not a direct navigation. I'd rather say a custom nav bar logic.
Replace the existing Compose Navigation library with the new Navigation 3 library.